Repository navigation
nvx-rs: document when a child joins its cgroup and what settlement counts - #922
Closed
Enrique Saurez (esaurez) wants to merge 1 commit into
Closed
Enrique Saurez (esaurez) wants to merge 1 commit into
Enrique Saurez (esaurez) wants to merge 1 commit into
Conversation
…unts A child that Command::cgroup places joins its cgroup in one of its own launch stages, which spawn does not wait for, and which follows the release of its barrier, if it waits at one. Until its launch report arrives, the cgroup may not hold the child, so an occupancy check, a kill, a removal or a settlement in that window can miss it. Say so on Command::cgroup, with the two ways to have the child in the cgroup first, and point to it from ExecutionCgroup::placement, is_occupied and kill, as remove already does. The execution binding settles on ExecutionCgroup::is_occupied, which reads pids.current. pids.current counts a zombie until its parent collects it, while cgroup.procs and the populated entry of cgroup.events drop a process as soon as it exits, so a caller that watches those files can see the cgroup empty before the execution settles. Say so in the binding's module documentation. Pin the order that the documentation now states with a test: a child spawned with a barrier and a cgroup has not joined the cgroup while it waits at its barrier, and has joined once its launch report arrives. Nothing else changes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: e861ffac-4aa0-4c85-9638-6aa98a9c48a3
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The documentation matches the implementation, the regression test checks the stated ordering, and no blocking issues remain.
No open issues 🎉
What changed in this PR
This PR clarifies the guest library’s cgroup placement and settlement guarantees without changing runtime behavior, supporting the broader cgroup work in #912.
Changes:
- Documents that child-side cgroup placement follows barrier release and is not awaited by
spawn. - Explains that settlement uses
pids.current, which counts zombies until reaped. - Adds a regression test verifying cgroup placement occurs after barrier release and before the launch report arrives.
| File | Description |
|---|---|
nvx-rs/src/guest/spawn.rs |
Documents placement timing and adds a barrier-ordering regression test. |
nvx-rs/src/guest/exec/binding.rs |
Clarifies settlement accounting and how it differs from live-process indicators. |
nvx-rs/src/guest/cgroup.rs |
Documents placement timing and its effect on occupancy checks and kills. |
Review effort: Balanced
Enrique Saurez (esaurez)
marked this pull request as ready for review
October 27, 2026 03:07
Enrique Saurez (esaurez)
deleted the
esaurez/exec-cgroup-docs-20261026
branch
November 3, 2026 22:06
Enrique Saurez (esaurez)
restored the
esaurez/exec-cgroup-docs-20261026
branch
November 3, 2026 22:21
Enrique Saurez (esaurez)
changed the base branch from
dev
to
feature/nvx-refactor
November 6, 2026 00:36
Enrique Saurez (esaurez)
changed the base branch from
feature/nvx-refactor
to
dev
November 6, 2026 02:21
Pedro Henrique Penna (ppenna)
deleted the
esaurez/exec-cgroup-docs-20261026
branch
November 6, 2026 17:16
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two facts that callers of the guest library rely on hold today, but the documentation doesn't state them where callers look:
Command::cgroup, the child writes its process ID to the cgroup'scgroup.procsin itsCgrouplaunch stage, after it has placed its descriptors and, if it waits at a barrier, after the barrier's release (PreparedChild::run_childinnvx-rs/src/guest/sys/spawn.rs).Command::spawnreturns once it has started the child, without waiting for that stage. So until the child'sLaunchReportarrives, the cgroup may not hold the child, andExecutionCgroup::is_occupied,kill,removeandsettlecan miss it.ExecutionCgroup::is_occupied, which readspids.current, finds the cgroup empty.pids.currentcounts a zombie until its parent collects it, whilecgroup.procsand thepopulatedentry ofcgroup.eventsdrop a process as soon as it exits. So a caller that watches those files can see the cgroup empty before the execution settles.This PR documents both, and changes no behavior:
Command::cgroup: the window in which the cgroup may not hold the child, and the two ways for a caller to have the child in the cgroup first: wait for the launch report, or spawn the child with a barrier instead of the cgroup, place it withExecutionCgroup::place, and then release the barrier.ExecutionCgroup::placement: the child joins in one of its own launch stages, with a link toCommand::cgroup.ExecutionCgroup::is_occupiedandkill: a child spawned with the placement whose launch has yet to report may not be in the cgroup yet, worded likeremove's existing warning.nvx::guest::exec::binding's module documentation: settlement checksExecutionCgroup::is_occupied, which readspids.current, andpids.currentcounts a zombie until its parent collects it, unlikecgroup.procsandpopulated.guest::spawn::tests::a_child_joins_its_cgroup_once_its_barrier_is_released, pins the order that the documentation now states, with the fake membership file of the existing cgroup test. A child spawned with a barrier and a cgroup has not joined while it waits at its barrier, and has joined once its launch report arrives. The test knows that the child waits once a pipe hangs up whose writer the child closes just before it waits, so it fails whenever the child joins before its barrier, however the child is scheduled.nvx::guest::cgroup's module documentation stays as it is: #912 rewrites it with its findings.Refs #912
Kernel references (Linux v6.18)
Documentation/admin-guide/cgroup-v2.rst: "After exit, a process stays associated with the cgroup that it belonged to at the time of exit until it's reaped; however, a zombie process does not appear in "cgroup.procs" [...]". A cgroup "which doesn't have any children and is associated only with zombie processes is considered empty and can be removed".cgroup.events:populatedis "1 if the cgroup or its descendants contains any live processes; otherwise, 0."do_exit()callscgroup_exit(), which takes the task off its css_set and updatespopulated, beforeexit_notify()makes it a zombie.cgroup.procsthen skips it (css_task_iter_advance()).pidscontroller uncharges a task inpids_release(), its.releasecallback, whichcgroup_release()calls fromrelease_task(), once the zombie's parent collects it.Validation
On a Linux x86-64 host, in
nvx-rs, withNVX_BUNDLE=skipand Rust 1.95.0 unless noted:cargo fmt --all --checkcargo clippy --locked --all-targets --all-features -- -D warningscargo clippy --locked --lib -- -D warningscargo clippy --locked --lib --no-default-features -- -D warningscargo test --locked --all-features --lib -- guest::spawn guest::cgroup guest::exec, thencargo test --locked --all-featuresguest::spawntests 5 timesRUSTDOCFLAGS='-D warnings' cargo doc --locked --no-deps --all-featurescargo +1.89 check --locked --all-targets --all-featurespython3 scripts/nvx.py check-guest-binaries, from the repository's root: the musl Clippy and tests of the guest libraryOn Windows:
python scripts\nvx.py verify.No VM backend ran: the change is documentation and one unit test. CI runs the rest of check-nvx-rs, check-aci-edge-sandboxes, and the KVM, MSHV and WHP jobs.